Stop unresolved templates before they reach a tool (#153) - #439
Merged
Conversation
Measured before this change, on a pipeline whose `content` referenced a
step that does not exist:
$ orchestrator run bad_template.yaml
"success": true, "error": null
$ cat out/bad.txt
value = {{ nonexistent_step.result.field }}
The literal Jinja source on disk, in a file a user would read as output,
from a run that exited reporting no error. `validate` refused the same
document -- so this is the mirror image of #241, with the validator right
and the executor wrong.
Root cause: TemplateManager catches UndefinedError, logs a warning and
returns the original template text. That fallback is deliberate and has
to stay, because resolution runs in several passes and a reference that
cannot resolve yet may resolve later. What was missing is a point where
the fallback stops being acceptable.
That point is resolve_before_tool_execution, which already promised
"tool parameters with all templates resolved" and is the last place the
promise can be kept -- past it the value is written to a file or sent to
a model. It is also the single funnel: Tool.execute routes every tool
through it. The check now enforces what the docstring claimed.
The test is *survival*, not presence. A marker in the output that was not
in the input is content: Jinja's own escape `{{ '{{' }}` renders to a
literal `{{`, and a step result may legitimately contain text that looks
like a template. A first version compared only the output and rejected
escaped Jinja -- caught by its own test, which is why the rule is now
"appeared in the input and came back byte-identical".
Nested containers are walked, since a parameter is routinely a list of
paths or a dict of fields and a reference buried in one reaches a tool
just as readily. `_`-prefixed parameters are skipped: those are plumbing
the control systems attach on the way to a tool, not authored values.
Rendering turns out to be all-or-nothing per string -- StrictUndefined
aborts the whole render -- so every marker in a failed string is
reported, not only the undefined one. Reporting just the bad reference
would imply the others had been substituted.
ADR 0001 now states the contract, alongside the action registry from
#241, since neither was written down.
Blocking suite 435 -> 456 passed, 13 skipped, 0 failed, 0 xfailed.
Collection 3230 -> 3251, 0 warnings. lock, ruff on src, and
`git diff --check` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #153.
Measured before
Literal Jinja source on disk, in a file a user would read as output, from a run that exited reporting no error.
validaterefused the same document — so this is the mirror image of #241, with the validator right and the executor wrong.Measured after
Exit 1, no artifact, the reference named.
Where the fix goes, and why not where the bug is
TemplateManagercatchesUndefinedError, logs a warning, and returns the original template text (template_manager.py:516). That fallback is deliberate and has to stay — resolution runs in several passes, and a reference that cannot resolve yet may resolve later. Re-raising there would break legitimate deferred resolution.What was missing is a point where the fallback stops being acceptable. That is
resolve_before_tool_execution, which already promised "tool parameters with all templates resolved" and is the last place the promise can be kept: past it the value is written to a file or sent to a model. It is also the single funnel —Tool.executeroutes every tool through it (tools/base.py:40), so this is one edit rather than one per output-writing tool.The check now enforces what the docstring already claimed.
Survival, not presence
A first version compared only the rendered output and flagged any
{{ }}it found. That rejected escaped Jinja:{{ '{{' }}renders to a literal{{, so writing documentation about templates became impossible. Its own test caught it.The rule is now: flag a marker only if it appeared in the input and came back byte-identical. A marker present in the output but not the input is content — escaped delimiters, or a step result that legitimately contains template-looking text (a scraped page, a code sample). Both cases have tests.
One bad reference takes the whole string down
StrictUndefinedaborts the entire render, so in"{{ here }} but not {{ missing }}"the resolvable{{ here }}does not survive either. Both markers are reported.I initially asserted only
{{ missing }}would be reported, and the test failed — the code was right and my expectation was wrong. Reporting just the undefined reference would imply the others had been substituted. This behaviour is now pinned and documented, because a user hitting it would otherwise find it baffling.Coverage
tests/test_unresolved_templates.py— 21 tests:{% if %}blocks caught, not just{{ }}_-prefixed plumbing parameters not inspectedNumbers
uv lock --check/ ruff onsrc/git diff --checkADR 0001 now states this contract and the action registry from #241; neither was written down.
Note on scope
This closes the reported defect at the tool boundary. It does not make an unresolved reference a compile-time error —
validatealready rejects this pipeline, so the two now agree, but a reference that only becomes resolvable at runtime is still legal by design. Tightening that further is a schema decision, not a bug fix.